Gdb string read (#364)

* harden gdb reader to long string input.

fix bug in the gdb reader that resulted in file position being lost
if a string was truncated when reading due to the finite buffer size.

fix bug in the gdb reader that could result in data being written outside
the provided buffer.

enhance gdb readers to remove length limit on reading strings.

* whitespace correction.

diff --git a/gbfile.cc b/gbfile.cc
index 47401d9965cbbfa4716bb4cb7f3894e045f52894..bafb46865d6eca8d3b0f850c385b02091249d057 100644 (file)
--- a/gbfile.cc
+++ b/gbfile.cc
@@ -20,13 +20,22 @@
 
  */
 
+#include <QtCore/QByteArray>   // for QByteArray
+#include <QtCore/QString>      // for QString
+#include <QtCore/QtGlobal>     // for qPrintable
+
+#include <cassert>             // for assert
+#include <cstdarg>             // for va_list, va_end, va_copy, va_start
+#include <cstdio>              // for EOF, ferror, ftell, SEEK_SET, SEEK_CUR, SEEK_END, clearerr, fclose, feof, fflush, fileno, fread, fseek, fwrite, ungetc, vsnprintf, FILE, stdin, stdout
+#include <cstring>             // for memcpy, strlen, strchr, strcpy, strncat
+#include <ctype.h>             // for tolower
+#include <errno.h>             // for errno
+
 #include "defs.h"
 #include "gbfile.h"
 #include "src/core/logging.h"
 
-#include <cassert>
-#include <cstdarg> // for va_copy
-#include <cstdio>
+#include "cet.h"               // for cet_ucs4_to_utf8
 
 #if __WIN32__
 /* taken from minigzip.c (part of the zlib project) */
@@ -1001,6 +1010,15 @@ gbfgetcstr(gbfile* file)
   return rv; 
 }
 
+QByteArray
+gbfgetnativecstr(gbfile* file)
+{
+  char* result = gbfgetcstr_old(file);
+  QByteArray rv(result);
+  xfree(result);
+  return rv; 
+}
+
 /*
  * gbfgetpstr: Reads a pascal string (first byte is length) from file.
  *             The result is a temporary allocated entity: use it or free it!

diff --git a/gbfile.h b/gbfile.h
index 87fb7ef38d2a468a163de6c4d2fb23e697f1aa1f..6a0bea73cd6b34d6ee2e82dfd1fd95b18f1f6103 100644 (file)
--- a/gbfile.h
+++ b/gbfile.h
@@ -23,11 +23,15 @@
 #ifndef GBFILE_H
 #define GBFILE_H
 
+#include <QtCore/QByteArray>    // for QByteArray
+#include <QtCore/QString>       // for QString
+
+#include <cstdarg>             // for va_list
+#include <cstdio>              // for FILE
+#include <cstdint>             // for int32_t, int16_t, uint32_t
 
 #include "defs.h"
-#include "cet.h"
 
-#include <QtCore/QString>
 
 struct gbfile_s;
 typedef struct gbfile_s gbfile;
@@ -117,6 +121,7 @@ float gbfgetflt(gbfile* file);                      // read a float value
 char* gbfgetstr(gbfile* file);                 // read until any type of line-breaks or EOF
 QString gbfgetpstr(gbfile* file);              // read a pascal string
 QString gbfgetcstr(gbfile* file);              // read a null terminated string
+QByteArray gbfgetnativecstr(gbfile* file);  // read a null terminated string
 char* gbfgetcstr_old(gbfile* file);            // read a null terminated string
 
 int gbfputint16(int16_t i, gbfile* file);

diff --git a/gdb.cc b/gdb.cc
index a03fe763dba54cec506f4df8edcfc2641f922e71..8bf71d1c8b4741c50446c9d086a066105cfe10cf 100644 (file)
--- a/gdb.cc
+++ b/gdb.cc
@@ -175,7 +175,7 @@ disp_summary(const gbfile* f)
 #define FREAD_DBL gbfgetdbl(fin)
 #define FREAD_LATLON GPS_Math_Semi_To_Deg(gbfgetint32(fin))
 
-#define FREAD_STR(a) gdb_fread_str(a,sizeof(a),fin)
+#define FREAD_STR() gbfgetnativecstr(fin)
 
 // This is all very messy.  Some versions of GDB store strings as 
 // 8859-1 strings and others as UTF8.  This wrapper tries to hide 
@@ -251,24 +251,6 @@ gdb_fread_cstr(gbfile* fin)
   return result;
 }
 
-static int
-gdb_fread_str(char* buf, int size, gbfile* fin)
-{
-  char c;
-  int res = 0;
-
-  while (size--) {
-    gbfread(&c, 1, 1, fin);
-    buf[res] = c;
-    if (c == '\0') {
-      return res;
-    }
-    res++;
-  }
-  buf[res] = '\0';
-  return res;
-}
-
 static QString
 gdb_fread_strlist()
 {
@@ -443,11 +425,10 @@ read_file_header()
 
   int reclen = FREAD_i32;
   Q_UNUSED(reclen);
-  int i = FREAD_STR(buf);
-  Q_UNUSED(i);
-  is_fatal(buf[0] != 'D', MYNAME ": Invalid file \"%s\"!", fin->name);
+  QByteArray drec = FREAD_STR();
+  is_fatal(drec.at(0) != 'D', MYNAME ": Invalid file \"%s\"!", fin->name);
 
-  gdb_ver = buf[1] - 'k' + 1;
+  gdb_ver = drec.at(1) - 'k' + 1;
   is_fatal((gdb_ver < GDB_VER_MIN) || (gdb_ver > GDB_VER_MAX),
            MYNAME ": Unknown or/and unsupported GDB version (%d.0)!", gdb_ver);
 
@@ -456,8 +437,7 @@ read_file_header()
   }
 
   reclen = FREAD_i32;
-  i = FREAD(buf, reclen + 1);
-  Q_UNUSED(i);
+  (void) FREAD(buf, reclen + 1);
   if (global_opts.verbose_status > 0) {
     const char* name = buf+2;
     if (strstr(name, "SQA") == nullptr) {
@@ -468,8 +448,8 @@ read_file_header()
     warning(MYNAME ": File created with \"%s\"\n", name);
   }
 
-  i = FREAD_STR(buf);
-  is_fatal(!(((i == 9) && (strcmp(buf, "MapSource") == 0)) || ((i == 8) && (strcmp(buf, "BaseCamp") == 0))), MYNAME ": Not a recognized signature in header");
+  QByteArray applicationField = FREAD_STR();
+  is_fatal(!((applicationField == "MapSource") || (applicationField == "BaseCamp")), MYNAME ": Not a recognized signature in header");
 }
 
 /*-----------------------------------------------------------------------------*/
@@ -478,13 +458,13 @@ static Waypoint*
 read_waypoint(gt_waypt_classes_e* waypt_class_out)
 {
   char buf[128];               /* used for temporary stuff */
+  QByteArray ba;
   int display, icon;
   gt_waypt_classes_e wpt_class;
   int i;
   Waypoint* res;
   garmin_fs_t* gmsd;
   char* str;
-  char* bufp = buf;
 #ifdef GMSD_EXPERIMENTAL
   char subclass[22];
 #endif
@@ -506,8 +486,8 @@ read_waypoint(gt_waypt_classes_e* waypt_class_out)
     waypth_ct++;
   }
 
-  FREAD_STR(buf);                                      /* Country code */
-  GMSD_SETSTR(cc, bufp);
+  ba = FREAD_STR();                                    /* Country code */
+  GMSD_SETSTR(cc, ba.constData());
 
 #ifdef GMSD_EXPERIMENTAL
   FREAD(subclass, sizeof(subclass));
@@ -578,12 +558,12 @@ read_waypoint(gt_waypt_classes_e* waypt_class_out)
   FREAD_i32;                           /* color !not implemented! */
   icon = FREAD_i32;
   GMSD_SET(icon, icon);                        /* icon */
-  FREAD_STR(buf);                              /* city */
-  GMSD_SETSTR(city, bufp);
-  FREAD_STR(buf);                              /* state */
-  GMSD_SETSTR(state, bufp);
-  FREAD_STR(buf);                              /* facility */
-  GMSD_SETSTR(facility, bufp);
+  ba = FREAD_STR();                            /* city */
+  GMSD_SETSTR(city, ba.constData());
+  ba = FREAD_STR();                            /* state */
+  GMSD_SETSTR(state, ba.constData());
+  ba = FREAD_STR();                            /* facility */
+  GMSD_SETSTR(facility, ba.constData());
 
   FREAD(buf, 1);
 
@@ -627,8 +607,8 @@ read_waypoint(gt_waypt_classes_e* waypt_class_out)
 
     waypt_flag = 0;
 
-    FREAD_STR(buf);                            /* street address */
-    GMSD_SETSTR(addr, bufp);
+    ba = FREAD_STR();                          /* street address */
+    GMSD_SETSTR(addr, ba.constData());
 
     FREAD(buf, 5);                             /* instruction depended */
     res->description = FREAD_CSTR_AS_QSTR;     /* instruction */
@@ -686,14 +666,14 @@ read_waypoint(gt_waypt_classes_e* waypt_class_out)
   /* VERSION DEPENDENT CODE */
   if (gdb_ver >= GDB_VER_3) {
     if (FREAD_i32 == 1) {
-      FREAD_STR(buf);          /* phone number */
-      GMSD_SETSTR(phone_nr, bufp);
-      FREAD_STR(buf);          /* ?? fax / mobile ?? */
+      ba = FREAD_STR();                /* phone number */
+      GMSD_SETSTR(phone_nr, ba.constData());
+      (void) FREAD_STR();              /* ?? fax / mobile ?? */
     }
-    FREAD_STR(buf);                    /* country */
-    GMSD_SETSTR(country, bufp);
-    FREAD_STR(buf);                    /* postal code */
-    GMSD_SETSTR(postal_code, bufp);
+    ba = FREAD_STR();                  /* country */
+    GMSD_SETSTR(country, ba.constData());
+    ba = FREAD_STR();                  /* postal code */
+    GMSD_SETSTR(postal_code, ba.constData());
   }
 
   res->icon_descr = gt_find_desc_from_icon_number(icon, GDB);
@@ -769,7 +749,7 @@ read_route()
 
     wpt->shortname = fread_cstr();     /* shortname */
     int wpt_class = FREAD_i32;         /* waypoint class */
-    FREAD_STR(buf);                    /* country code */
+    (void) FREAD_STR();                        /* country code */
     FREAD(buf, 18 + 4);                /* subclass part 1-3 / unknown */
 
     if (FREAD_C != 0) {
@@ -1086,7 +1066,6 @@ read_data()
       break;
     }
 
-    fin = fsave;
     int delta = len - gbftell(ftmp);
     is_fatal(delta > 1000000, "Internal consistency error.  Delta too big");
 
@@ -1117,6 +1096,8 @@ read_data()
       }
       warning("\n");
     }
+
+    fin = fsave;
   }